Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include files from a source-directory if that source-directory is in elm-stuff #321

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

jfmengels
Copy link
Owner

Fixes #320 reported by @morteako

We now only ignore elm-stuff inside every source-directory, rather than all of them.
This includes a test that makes sure that those files get included in the analysis AND that we can report errors for them.

@lishaduck
Copy link
Contributor

Hm. The failing test isn't the flake. I assume it passes locally?

@jfmengels
Copy link
Owner Author

jfmengels commented Nov 29, 2024

Correct, yes. Well, at least that new test passes. This is symptomatic of elm-stuff being ignored/absent in CI (I can tell because that's the error I was trying to fix) but I don't know what the issue could be 🤔

@jfmengels
Copy link
Owner Author

jfmengels commented Nov 29, 2024

Oh, it's because a file did not committed. I somehow made an exclusion to it in .gitignore, but it still doesn't want to get included. I'm investigating why.

EDIT: Success. I couldn't get it to be included by editing the .gitignore file (I still don't understand why), but I forced the file to be added by doing git add <file> -f.

@jfmengels jfmengels merged commit 93c3ef8 into main Nov 29, 2024
3 checks passed
@jfmengels jfmengels deleted the elm-stuff-ignore branch November 29, 2024 21:10
@lishaduck
Copy link
Contributor

Oh, it's because a file did not committed. I somehow made an exclusion to it in .gitignore, but it still doesn't want to get included. I'm investigating why.

EDIT: Success. I couldn't get it to be included by editing the .gitignore file (I still don't understand why), but I forced the file to be added by doing git add <file> -f.

If I recall correctly, you can't unignore folder contents if the folder is ignored, so rather than blanket ignoring elm-stuff, you'd want to ignore **/elm-stuff/** (and you'd need the leading **/ because it's a non-trivial pattern—we love glob syntax 🤣)

@morteako
Copy link

morteako commented Dec 2, 2024

@jfmengels Seems like this also fixes the same issue but for NoUnused.Dependencies (which makes sense), which is great 💯

@jfmengels
Copy link
Owner Author

If I recall correctly, you can't unignore folder contents if the folder is ignored, so rather than blanket ignoring elm-stuff, you'd want to ignore **/elm-stuff/** (and you'd need the leading **/ because it's a non-trivial pattern—we love glob syntax 🤣)

Oh right... I knew that because I modeled the extra files API based on that, but my version makes that a bit more explicit and I forgot about the original 😅
But something weird happened locally, because even by removing the elm-stuff line, it didn't ignore elm-stuff. I don't know what happened but problem solved so 🤷

Seems like this also fixes the same issue but for NoUnused.Dependencies (which makes sense), which is great 💯

That's great @morteako! I'll try to publish in a few days, I hope this is not blocking you (please tell me if it is).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NoUnused.Variables reports wrong alias as unused when importing module from elm-git-install dependency
3 participants